NO-ISSUE: Synchronize From Upstream Repositories#694
NO-ISSUE: Synchronize From Upstream Repositories#694openshift-bot wants to merge 93 commits intoopenshift:mainfrom
Conversation
…ity (#2622) Rename "ref Secrets/refs" step definitions to "referred secrets" for better readability. Update the labels step to accept a data table and extract a shared matchLabels helper with deterministic key ordering. Add diagnostic logging when label matching fails during polling. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…ace (#2624) The controller cache only watches the system namespace, causing ref resolution to fail when Secrets are stored in other namespaces. Fix by introducing a client wrapper that falls back to direct API calls for Secret reads outside the system namespace, and grant cluster-wide Secret get permission when BoxcutterRuntime is enabled. Adds an e2e scenario covering this path. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Bumps [github.com/klauspost/compress](https://github.com/klauspost/compress) from 1.18.4 to 1.18.5. - [Release notes](https://github.com/klauspost/compress/releases) - [Commits](klauspost/compress@v1.18.4...v1.18.5) --- updated-dependencies: - dependency-name: github.com/klauspost/compress dependency-version: 1.18.5 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
|
@openshift-bot: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@openshift-bot: GitHub didn't allow me to request PR reviews from the following users: openshift/openshift-team-operator-framework. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (28)
📒 Files selected for processing (14)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (6)
WalkthroughAdded a secret-fallback cached client, removed external kubeconfig flags, widened RBAC to allow secret GETs, refactored e2e tests to validate objects stored in Secrets, updated Mozilla TLS profiles and curve support, and bumped a few dependencies. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: openshift-bot The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
1 similar comment
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: openshift-bot The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/steps/steps.go`:
- Around line 313-322: The code unconditionally falls back to sc.namespace when
res.GetNamespace() is empty, misclassifying cluster-scoped objects; change the
logic so the fallback to sc.namespace only happens for namespaced resource
kinds. Specifically, after reading res.GetNamespace() and res.GetKind(),
determine whether the kind is namespaced (using your existing
RESTMapper/discovery helper or add a small isNamespaced(kind) check), and only
set namespace = sc.namespace when that check returns true; otherwise leave
namespace as empty string before appending to sc.addedResources (referencing
res.GetNamespace(), res.GetKind(), sc.namespace, sc.addedResources and the
resource struct).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3dd1d555-751e-4aa0-8058-a07dd0acf7fe
⛔ Files ignored due to path filters (25)
go.sumis excluded by!**/*.sumopenshift/tests-extension/go.sumis excluded by!**/*.sumvendor/github.com/klauspost/compress/.goreleaser.ymlis excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/flate/huffman_code.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/flate/regmask_other.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/huff0/decompress_amd64.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/huff0/decompress_generic.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/internal/cpuinfo/cpuinfo_amd64.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/blockenc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/enc_base.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/enc_best.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/enc_better.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/enc_dfast.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/enc_fast.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/encoder.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/encoder_options.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder_amd64.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder_generic.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/xxhash_other.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/matchlen_amd64.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/matchlen_generic.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_amd64.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_generic.gois excluded by!vendor/**,!**/vendor/**vendor/modules.txtis excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (9)
cmd/operator-controller/main.gogo.modhelm/olmv1/templates/rbac/clusterrole-operator-controller-manager-role.ymlmanifests/experimental-e2e.yamlmanifests/experimental.yamltest/e2e/features/install.featuretest/e2e/features/revision.featuretest/e2e/steps/hooks.gotest/e2e/steps/steps.go
| } else { | ||
| namespace := res.GetNamespace() | ||
| if namespace == "" { | ||
| namespace = sc.namespace | ||
| } | ||
| sc.addedResources = append(sc.addedResources, resource{ | ||
| name: res.GetName(), | ||
| kind: strings.ToLower(res.GetKind()), | ||
| namespace: namespace, | ||
| }) |
There was a problem hiding this comment.
Do not default empty namespace for cluster-scoped resources.
At Line 315-Line 317, resources with no metadata.namespace are always assigned sc.namespace. That misclassifies cluster-scoped objects as namespaced; cleanup then issues kubectl delete ... -n <ns> and can fail, leaving leaked resources between scenarios.
Please gate the fallback so it applies only to namespaced resources, and keep namespace empty for cluster-scoped ones.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/e2e/steps/steps.go` around lines 313 - 322, The code unconditionally
falls back to sc.namespace when res.GetNamespace() is empty, misclassifying
cluster-scoped objects; change the logic so the fallback to sc.namespace only
happens for namespaced resource kinds. Specifically, after reading
res.GetNamespace() and res.GetKind(), determine whether the kind is namespaced
(using your existing RESTMapper/discovery helper or add a small
isNamespaced(kind) check), and only set namespace = sc.namespace when that check
returns true; otherwise leave namespace as empty string before appending to
sc.addedResources (referencing res.GetNamespace(), res.GetKind(), sc.namespace,
sc.addedResources and the resource struct).
|
/test e2e-aws-techpreview-olmv1-ext |
Bumps [github.com/go-jose/go-jose/v4](https://github.com/go-jose/go-jose) from 4.1.3 to 4.1.4. - [Release notes](https://github.com/go-jose/go-jose/releases) - [Commits](go-jose/go-jose@v4.1.3...v4.1.4) --- updated-dependencies: - dependency-name: github.com/go-jose/go-jose/v4 dependency-version: 4.1.4 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [mkdocs-material](https://github.com/squidfunk/mkdocs-material) from 9.7.5 to 9.7.6. - [Release notes](https://github.com/squidfunk/mkdocs-material/releases) - [Changelog](https://github.com/squidfunk/mkdocs-material/blob/master/CHANGELOG) - [Commits](squidfunk/mkdocs-material@9.7.5...9.7.6) --- updated-dependencies: - dependency-name: mkdocs-material dependency-version: 9.7.6 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
260a349 to
2c564e0
Compare
|
New changes are detected. LGTM label has been removed. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/e2e/steps/steps.go (1)
781-789:⚠️ Potential issue | 🟠 MajorKeep namespace information with referred Secret refs.
cmd/operator-controller/main.goLines 809-813 explicitly added support for Secret refs outside the system namespace, but these helpers still flatten refs down to names and then list onlyolmNamespace. The steps built on them can miss non-system refs or validate a same-named Secret in the wrong namespace, which weakens coverage for the regression this PR is fixing. Collect{name, namespace}fromspec.phases[].objects[].ref, default empty namespace as needed, and fetch those Secrets directly.Also applies to: 905-939, 944-955
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/steps/steps.go` around lines 781 - 789, The test helpers currently call collectReferredSecretNames which returns only secret names and then call getResource("secret", name, namespace) against a single namespace, causing misses for Secrets referenced with non-system namespaces; change the helpers to collect and return structs of {name, namespace} from spec.phases[].objects[].ref (default namespace to empty string when not set), update places using collectReferredSecretNames (and loops at the other mentioned ranges) to iterate over each secretRef and call getResource("secret", secretRef.name, secretRef.namespace) (or the empty/default where appropriate) and adjust waitFor usages accordingly so the validation fetches the correct namespace for each Secret.
♻️ Duplicate comments (1)
test/e2e/steps/steps.go (1)
313-322:⚠️ Potential issue | 🟠 MajorDon't default empty namespaces for cluster-scoped objects.
When
metadata.namespaceis empty on Line 315, this still assignssc.namespacefor every non-ClusterExtension/non-ClusterObjectSet. That misclassifies resources likeNamespaceorClusterRoleBinding, andScenarioCleanupintest/e2e/steps/hooks.goLines 199-201 then deletes them with-n <ns>, which can leak resources between scenarios.Possible fix
} else { namespace := res.GetNamespace() - if namespace == "" { + if namespace == "" && isNamespacedResource(res) { namespace = sc.namespace } sc.addedResources = append(sc.addedResources, resource{🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/steps/steps.go` around lines 313 - 322, The code currently defaults empty metadata.namespace to sc.namespace for every object before appending to sc.addedResources (res.GetNamespace()), which mislabels cluster-scoped resources; change the logic where sc.addedResources is appended (the block around res.GetNamespace() and sc.addedResources) to only default to sc.namespace when the resource kind is namespace-scoped — e.g., add a helper or inline check (isClusterScoped/isNamespaceScoped) for res.GetKind() and if the kind is cluster-scoped (Namespace, ClusterRole, ClusterRoleBinding, ClusterExtension, ClusterObjectSet, etc.) leave namespace as empty string; otherwise set namespace = sc.namespace and then append the resource. Ensure ScenarioCleanup (hooks.go) will therefore not delete cluster-scoped objects using a -n flag.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@test/e2e/steps/steps.go`:
- Around line 781-789: The test helpers currently call
collectReferredSecretNames which returns only secret names and then call
getResource("secret", name, namespace) against a single namespace, causing
misses for Secrets referenced with non-system namespaces; change the helpers to
collect and return structs of {name, namespace} from spec.phases[].objects[].ref
(default namespace to empty string when not set), update places using
collectReferredSecretNames (and loops at the other mentioned ranges) to iterate
over each secretRef and call getResource("secret", secretRef.name,
secretRef.namespace) (or the empty/default where appropriate) and adjust waitFor
usages accordingly so the validation fetches the correct namespace for each
Secret.
---
Duplicate comments:
In `@test/e2e/steps/steps.go`:
- Around line 313-322: The code currently defaults empty metadata.namespace to
sc.namespace for every object before appending to sc.addedResources
(res.GetNamespace()), which mislabels cluster-scoped resources; change the logic
where sc.addedResources is appended (the block around res.GetNamespace() and
sc.addedResources) to only default to sc.namespace when the resource kind is
namespace-scoped — e.g., add a helper or inline check
(isClusterScoped/isNamespaceScoped) for res.GetKind() and if the kind is
cluster-scoped (Namespace, ClusterRole, ClusterRoleBinding, ClusterExtension,
ClusterObjectSet, etc.) leave namespace as empty string; otherwise set namespace
= sc.namespace and then append the resource. Ensure ScenarioCleanup (hooks.go)
will therefore not delete cluster-scoped objects using a -n flag.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1c3ce0ed-d390-4fd4-96de-fd0f24edd78f
⛔ Files ignored due to path filters (28)
go.sumis excluded by!**/*.sumopenshift/tests-extension/go.sumis excluded by!**/*.sumvendor/github.com/go-jose/go-jose/v4/asymmetric.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-jose/go-jose/v4/cipher/key_wrap.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-jose/go-jose/v4/symmetric.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/.goreleaser.ymlis excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/flate/huffman_code.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/flate/regmask_other.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/huff0/decompress_amd64.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/huff0/decompress_generic.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/internal/cpuinfo/cpuinfo_amd64.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/blockenc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/enc_base.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/enc_best.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/enc_better.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/enc_dfast.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/enc_fast.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/encoder.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/encoder_options.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder_amd64.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder_generic.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/xxhash_other.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/matchlen_amd64.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/matchlen_generic.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_amd64.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_generic.gois excluded by!vendor/**,!**/vendor/**vendor/modules.txtis excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (10)
cmd/operator-controller/main.gogo.modhelm/olmv1/templates/rbac/clusterrole-operator-controller-manager-role.ymlmanifests/experimental-e2e.yamlmanifests/experimental.yamlrequirements.txttest/e2e/features/install.featuretest/e2e/features/revision.featuretest/e2e/steps/hooks.gotest/e2e/steps/steps.go
✅ Files skipped from review due to trivial changes (1)
- requirements.txt
🚧 Files skipped from review as they are similar to previous changes (5)
- helm/olmv1/templates/rbac/clusterrole-operator-controller-manager-role.yml
- go.mod
- manifests/experimental.yaml
- test/e2e/features/install.feature
- test/e2e/features/revision.feature
This reverts commit 8ccea5a.
2c564e0 to
81d34b3
Compare
|
New changes are detected. LGTM label has been removed. |
|
Hi @Xia-Zhao-rh , could you help verify it? Thanks! |
|
/verified by @Xia-Zhao-rh |
The test operator's httpd script uses python3's http.server which binds to 0.0.0.0 (IPv4 only) by default. On IPv6-only networks (e.g. metal-ipi-ovn-ipv6-techpreview), the startup/liveness/readiness probes connect to the pod's IPv6 address but nothing is listening, causing the operator pod to never become Ready and the OLMv1 ClusterExtension install test to time out. Adding --bind :: makes python3 http.server listen on all interfaces including IPv6, fixing the test on dual-stack and IPv6-only clusters. This resolves the 0% pass rate on: - periodic-ci-openshift-release-main-nightly-4.22-e2e-metal-ipi-ovn-ipv6-techpreview Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…g for CE install tests With BoxcutterRuntime, Installed=True is only set after all availability probes pass, which can take longer on TechPreview clusters (IPv6, multi-arch). Increases install-specific timeout from 5m to 10m and logs condition state on each poll to aid debugging flaky failures.
Add 7 Ginkgo tests under [sig-olmv1][OCPFeatureGate:NewOLMDeploymentConfig] covering the spec.config.inline.deploymentConfig feature: Positive tests (verify applied customisations): - environment variables - resource requirements - tolerations - node selector - annotations on deployment and pod template Negative tests (verify terminal validation errors): - invalid deploymentConfig.env type (string instead of array) - unknown field inside deploymentConfig (additionalProperties:false) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Todd Short <tshort@redhat.com>
Signed-off-by: Todd Short <tshort@redhat.com>
…64 support Signed-off-by: Daniel Franz <dfranz@redhat.com>
Signed-off-by: Todd Short <tshort@redhat.com>
…t in OTE tests Update all remaining references to ClusterExtensionRevision in openshift/tests-extension to use ClusterObjectSet, matching the upstream rename in operator-framework/operator-controller#2589. Files updated: - test/qe/specs/olmv1_ce.go: RBAC resource names and comments - test/olmv1-preflight.go: scenario constants, test names, RBAC rules - .openshift-tests-extension/openshift_payload_olmv1.json: test name - pkg/bindata/qe/bindata.go: embedded RBAC templates - test/qe/testdata/olm/sa-nginx-limited-boxcutter.yaml: RBAC resources - test/qe/testdata/olm/sa-nginx-insufficient-operand-rbac-boxcutter.yaml: RBAC resources Signed-off-by: Camila Macedo <cmacedo@redhat.com> Made-with: Cursor
…s ClusterObjectSet The upstream rename of ClusterExtensionRevision to ClusterObjectSet (operator-framework/operator-controller#2589) breaks the incompatible operator detection in cluster-olm-operator. The cluster-olm-operator binary still reads ClusterExtensionRevision resources to find operators with olm.maxOpenShiftVersion, so after the rename it never detects incompatible operators and InstalledOLMOperatorsUpgradeable stays True. Skip this test when NewOLMBoxCutterRuntime feature gate is enabled until cluster-olm-operator is updated to read ClusterObjectSet. Signed-off-by: Camila Macedo <cmacedo@redhat.com> Made-with: Cursor
Signed-off-by: Francesco Giudici <fgiudici@redhat.com>
81d34b3 to
bff5883
Compare
|
@openshift-bot: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
New changes are detected. LGTM label has been removed. |
|
@openshift-bot: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/retest |
The downstream repository has been updated with the following following upstream commits:
The
vendor/directory has been updated and the following commits were carried:@catalogd-updateThis pull request is expected to merge without any human intervention. If tests are failing here, changes must land upstream to fix any issues so that future downstreaming efforts succeed.
/cc @openshift/openshift-team-operator-framework